Skip to content

feat: add auto-fix option for automatic clang-format fixing - #443

Open
shenxianpeng wants to merge 23 commits into
mainfrom
feature/auto-fix
Open

shenxianpeng wants to merge 23 commits into
mainfrom
feature/auto-fix

Conversation

@shenxianpeng

@shenxianpeng shenxianpeng commented Jun 8, 2026

Copy link
Copy Markdown
Member

Closes #439

Adds an auto-fix input. When enabled, cpp-linter runs with --fix (clang-format -i on the files with style issues) and the action commits the result to the branch. Fixes respect lines-changed-only.

Inputs

Input Default Purpose
auto-fix false Apply and commit clang-format fixes
auto-fix-commit-msg style: apply clang-format fixes Commit message
auto-fix-git-user $GITHUB_ACTOR Commit author name
auto-fix-git-email $GITHUB_ACTOR_ID+$GITHUB_ACTOR@users.noreply.github.com Commit author email

How it works

  • Runs only when auto-fix is enabled. Pull requests from forks are skipped with a warning: the token cannot push to the fork's branch.
  • On pull_request events the default checkout is the merge commit, and a fix committed on it would carry that merge into the branch. The action therefore checks out the head commit itself before linting, so workflows keep the default actions/checkout. It does this only when HEAD is the merge commit, never forces, and fetches the head SHA only if it is missing, because a --depth fetch truncates a full clone. The commit step verifies HEAD == head.sha and skips with a warning otherwise. On push events it commits to the pushed branch; tags and other refs are skipped.
  • Stages the modified tracked files matching the extensions input, then counts what is staged with git status --short. Files an earlier step changed and untracked sources stay out of the commit, and a .gitattributes LF/CRLF normalization cannot lead to an empty commit.
  • Pushes HEAD:refs/heads/<branch>. A rejected push is a warning with the git error and the permissions hint, not a failure.

Requires cpp-linter >= 1.14.0, which ships --fix; the pin on this branch is 1.14.0.

Docs

  • README.md: auto-fix section with a note that the action checks out the pull request head, [skip ci] tip, and a section on running the action with your own GitHub App token
  • docs/permissions.md: contents: write, the same checkout note, the limits (default GITHUB_TOKEN does not trigger CI; forks), and a "GitHub App token" chapter
  • docs/examples/index.md + .github/workflows/examples/auto-fix.yml: recipe
  • docs/action.yml: minimum version 2.23.0 for the four inputs

Tests

  • self-test.yml gains a test-auto-fix job that runs the action with auto-fix on the demo sources and asserts the commit (message, author, only files under docs/examples/demo, clean tree, zero remaining clang-format findings). It has no push credentials, so the push is rejected by design and the last step checks the commit never reached the PR branch. It uses the default checkout, so the action's own checkout of the pull request head is covered, and it is skipped on fork pull requests, where auto-fix makes no commit.
  • End to end on a real pull request with the default checkout: test: auto-fix pull-request e2e (do not merge) test-cpp-linter-action#61 started from the merge commit, the action switched to the pull request head and pushed a fix commit with the expected message and author. The full clone was left intact and clang-tidy's diagnostic count was unchanged by the fix (run).

Summary by CodeRabbit

  • New Features

    • Added optional automatic clang-format fixes for C/C++ pull requests.
    • Added configurable commit messages and author details for generated formatting commits.
    • Added an example workflow that applies fixes and fails when formatting checks remain unresolved.
  • Documentation

    • Documented configuration, required permissions, commit behavior, and fork limitations.
    • Added guidance for GitHub App tokens, pull request workflow reruns, and [skip ci] commit tags.
    • Added a recipe demonstrating automatic clang-format fixes.

@github-actions github-actions Bot added bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request labels Jun 8, 2026
2bndy5

This comment was marked as resolved.

Comment thread action.yml
@2bndy5 2bndy5 removed the bug Something isn't working label Jun 9, 2026
@github-actions github-actions Bot added the bug Something isn't working label Jun 9, 2026
Comment thread docs/permissions.md Outdated
@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-format (v16.0.6) reports: 2 file(s) not formatted
  • docs/examples/demo/demo.hpp
  • docs/examples/demo/demo.cpp
clang-tidy (v16.0.6) reports: 7 concern(s)

Have any feedback or feature suggestions? Share it here.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an auto-fix capability to the cpp-linter composite action so clang-format issues can be automatically fixed, committed, and pushed back to the PR branch, reducing the manual “fix + repush” workflow.

Changes:

  • Introduces new action inputs auto-fix and auto-fix-commit-msg, and conditionally passes --fix to cpp-linter.
  • Adds an auto-commit + push step to publish formatting fixes back to the PR branch.
  • Updates documentation and adds a new example workflow demonstrating auto-fix usage and required permissions.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
README.md Documents the new auto-fix feature with usage and [skip ci] guidance.
docs/permissions.md Documents the additional contents: write permission required for auto-fix.
docs/examples/index.md Adds an auto-fix recipe entry linking to the new example workflow.
docs/action.yml Adds metadata for new inputs (min version + required permissions).
action.yml Implements auto-fix inputs, conditional --fix flag, and auto-commit/push logic.
.github/workflows/examples/auto-fix.yml New example workflow showing required permissions and configuration for auto-fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread action.yml Outdated
Comment thread action.yml
Comment thread action.yml Outdated
Comment thread .github/workflows/examples/auto-fix.yml Outdated
Comment thread docs/permissions.md Outdated
Comment thread README.md Outdated
@shenxianpeng
shenxianpeng force-pushed the feature/auto-fix branch 2 times, most recently from fdfa0db to e3fd91b Compare July 24, 2026 07:22
@shenxianpeng
shenxianpeng marked this pull request as ready for review July 24, 2026 07:44
@shenxianpeng
shenxianpeng requested a review from a team as a code owner July 24, 2026 07:44
@shenxianpeng
shenxianpeng requested review from 2bndy5 and removed request for a team July 24, 2026 07:44
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 7cdfc83e-7d91-4416-985d-c4fcf7d46df5

📥 Commits

Reviewing files that changed from the base of the PR and between b79d3ba and 754adb0.

📒 Files selected for processing (4)
  • .github/workflows/examples/auto-fix.yml
  • README.md
  • docs/action.yml
  • docs/permissions.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • README.md

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

Adds configurable clang-format auto-fixing to the action. The action supports pull request branch checkout, conditional --fix execution, automatic commit and push handling, workflow examples, and documentation for permissions and fork limitations.

Changes

Clang-format auto-fix

Layer / File(s) Summary
Auto-fix inputs and metadata
action.yml, docs/action.yml
Defines auto-fix, commit message, and Git identity inputs with defaults and documentation metadata.
Fix-enabled linter execution
action.yml
Checks out same-repository pull request branches when required and conditionally appends --fix to the linter invocation.
Auto-commit and push flow
action.yml
Detects formatting changes, commits them with configured or fallback identity values, and pushes the commit.
Workflow examples and documentation
.github/workflows/examples/auto-fix.yml, README.md, docs/examples/index.md, docs/permissions.md
Adds an auto-fix workflow example and documents usage, permissions, CI behavior, fork limitations, and GitHub App token setup.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #439 requires an auto-fix option that formats code and commits the changes to the pull request branch. action.yml adds auto-fix with a default of false, passes --fix when enabled, dete…
Out of Scope Changes check ✅ Passed The workflow example, permission guidance, token guidance, version metadata, and lines-changed-only clarification directly support the auto-fix feature in issue #439. The commit identity inputs and …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding an auto-fix option for automatic clang-format fixes.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/examples/auto-fix.yml:
- Around line 10-12: Remove the pull-requests: write permission from the
workflow permissions block, retaining contents: write for auto-fix commits. Keep
this example’s baseline permissions limited to the access it uses.

In `@action.yml`:
- Around line 539-553: Update the auto-fix flow around has_changes and git add
-A to avoid staging the entire workspace. Capture the pre-linter tracked-file
baseline and determine which paths were modified by formatting, then stage only
those formatter-produced paths before committing; alternatively require a clean
tree before running the formatter. Preserve unrelated generated or pre-existing
workspace changes.
- Around line 560-561: Update the auto-commit flow around branch and push_result
to run only for branch refs and pull-request runs, skipping tag-triggered
executions. Detect the ref type before constructing the destination branch, and
preserve the existing branch selection and push behavior for supported runs.
- Around line 460-468: Gate the auto-fix branch checkout and all subsequent
auto-fix commit/push behavior on the PR originating from the same repository as
the workflow repository, using the repository identity fields available in the
GitHub context. For fork pull requests, preserve the initial merge checkout,
skip the GITHUB_HEAD_REF fetch/checkout and auto-fix push path, and emit a
warning that auto-fix cannot update the fork’s head ref. Update the checkout
step and the nearby auto-fix commit/push logic rather than changing unrelated
behavior.

In `@docs/permissions.md`:
- Around line 87-90: Convert the YAML example under the permissions section to
the repository’s required indented code-block style by removing the fenced block
markers and indenting the example consistently. Preserve the existing
permissions contents unchanged.
- Around line 94-109: Update the CI re-triggering guidance near the auto-fix
commit message reference so [skip ci] is recommended only when pushing with a
PAT or GitHub App token; remove the implication that it is needed with the
default GITHUB_TOKEN, while preserving the existing token and auto-fix context.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 144d5494-4fd2-4310-ab8c-50e1e5426494

📥 Commits

Reviewing files that changed from the base of the PR and between 0b93f58 and e3fd91b.

📒 Files selected for processing (6)
  • .github/workflows/examples/auto-fix.yml
  • README.md
  • action.yml
  • docs/action.yml
  • docs/examples/index.md
  • docs/permissions.md

Comment thread .github/workflows/examples/auto-fix.yml Outdated
Comment thread action.yml Outdated
Comment thread action.yml Outdated
Comment thread action.yml Outdated
Comment thread docs/permissions.md
Comment thread docs/permissions.md Outdated
Comment thread action.yml Outdated
Comment thread README.md Outdated
Comment thread action.yml Outdated
shenxianpeng and others added 16 commits September 15, 2026 22:29
Fixes Nu parser error:
  Error: nu::parser::assignment_requires_mutable_variable
  '' needs to be a mutable variable to support append ['--fix']
  Use 'mut args' instead of 'let args'
Apply 2bndy5's review suggestion: use single git -c user.name=X -c user.email=Y commit
instead of separate git config + git commit. This avoids polluting the global git
configuration in the CI environment.
- docs/permissions.md: Add warning about GITHUB_TOKEN not triggering CI
  on auto-fix commits, with PAT workaround. Add warning about
  third-party fork PRs where auto-fix cannot work.
- action.yml: Add intelligent fork-detection in push failure warning
  (checks for 403/refused/permission errors and shows context-aware
  message referencing docs).
- .github/workflows/examples/auto-fix.yml: Add comment about PAT
  option for CI re-triggering.
Add configurable git identity for auto-fix commits to address 2bndy5's
concern about commit author reflecting the token owner.

- auto-fix-git-user: custom git username for the auto-fix commit
  (defaults to GITHUB_ACTOR)
- auto-fix-git-email: custom git email for the auto-fix commit
  (defaults to GITHUB_ACTOR_ID+GITHUB_ACTOR@users.noreply.github.com)
- When empty, falls back to GITHUB_ACTOR/GITHUB_ACTOR_ID based values
Co-authored-by: Xianpeng Shen <xianpeng.shen@gmail.com>
Address review feedback on the auto-fix flow:

- Gate the PR-branch checkout and the auto-commit/push steps to
  same-repository pull requests. Fork PRs previously hard-failed the
  checkout (the fork's branch isn't on origin) and could not push; now
  they are skipped with a warning.
- Guard against tag refs: compute the destination branch from the PR
  head ref or a pushed branch ref, and skip otherwise so we never push
  HEAD to refs/heads/<tag> and create a stray branch.
- Stage only tracked modifications with `git add -u` instead of
  `git add -A`, so unrelated untracked/generated files are not swept
  into the auto-fix commit.
- docs/permissions.md: clarify the write permission is for the
  actions/checkout token, and correct the `[skip ci]` guidance (it only
  matters for PAT/App-token pushes, since the default GITHUB_TOKEN push
  does not trigger CI anyway).
- Example workflow: downgrade `pull-requests: write` to `read` (the
  example uses no review/thread-comment feature; `read` is still needed
  for files-changed-only on pull_request events).
I would like to change the default clang-format commit message and leave "refactor: apply clang-tidy fixes" for clang-tidy in the futhure if we also support auto-fix for it

Co-authored-by: Xianpeng Shen <xianpeng.shen@gmail.com>
Two places said something the implementation does not do.

The README told everyone to tag the commit with `[skip ci]` to stop it
re-running CI, but the example above it checks out with the default
`GITHUB_TOKEN` -- which cannot start a workflow run at all, so there was
nothing to skip. docs/permissions.md already had this right and the two
contradicted each other. The tip now names the token that makes `[skip
ci]` meaningful and points at the permissions page for both setups.

`auto-fix`'s own description implied it rewrites whole files. It does
not: cpp-linter assembles range-aware args, so `lines-changed-only`
narrows what gets reformatted. That matters, because with it enabled a
file can come back from auto-fix still failing a whole-file
`.clang-format` check -- worth saying out loud rather than leaving to be
discovered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- permissions.md: new "GitHub App token" section (App permissions,
  create-github-app-token, pass the token to checkout and cpp-linter);
  the CI re-trigger note points at it instead of recommending a PAT
- fork note: fork pull requests get no secrets, so an App token or PAT
  cannot help there; auto-fix stays skipped
- README tip and example workflow updated to match
- docs/action.yml: auto-fix inputs land in 2.22.0, not 2.19.0
Co-authored-by: Brendan <2bndy5@gmail.com>
Give the GitHub App token setup its own section under Usage instead of
only a passing mention in the auto-fix tip -- it applies to every feature
and needs no server to host webhooks.

Also add the relative link refs for permissions-doc and app-token-doc to
docs/index.md, which includes the README; without them those links
rendered as literal text on the docs homepage.
Co-authored-by: Brendan <2bndy5@gmail.com>
Co-authored-by: Xianpeng Shen <xianpeng.shen@gmail.com>
Re-applies the action.yml part of 8580162, which was dropped when the
branch was rewritten.

- On pull_request events check out the PR head commit only when HEAD is
  the merge commit, without --force; a dirty tree or a failed checkout
  skips auto-fix with a warning instead (actions/checkout provides
  refs/pull/N/merge, so committing there would push that merge into the
  branch)
- Stage only modified files matching the configured `extensions` and
  count `git ls-files --modified` output instead of parsing an exit code
…kflow

On pull_request events actions/checkout provides refs/pull/N/merge, so
the action used to switch the workspace to the head commit itself. That
silently changes the tree every later step sees. Drop the step and make
the workflow responsible instead, the way git-auto-commit-action does:
document `ref: ${{ github.event.pull_request.head.sha }}` on
actions/checkout (head.sha rather than head_ref so fork PRs still check
out), and have the commit step verify HEAD == head.sha on pull_request,
skipping with a warning that names the missing input otherwise.
…docs

- one auto-fix step instead of two: the fork check is an early exit in
  the commit step, so the condition logic lives in one place
- push failures print stderr plus the permissions hint instead of
  guessing the cause from the message text
- shorter input description; README no longer restates it as a list;
  the two permissions-page warnings are one admonition
Adds a job that runs the action with auto-fix on the demo sources and
asserts the resulting commit: message, author and email from the inputs,
only files under docs/examples/demo, a clean tree afterwards and zero
clang-format findings. The job has no push credentials, so the push is
rejected by design and the last step checks the commit never reached the
PR branch.
@shenxianpeng

Copy link
Copy Markdown
Member Author

From the test PR cpp-linter/test-cpp-linter-action#61, it seems to work well.

Do you have any other comments, feedback, or concerns about this PR @2bndy5

@shenxianpeng
shenxianpeng requested a review from 2bndy5 September 16, 2026 11:23
@shenxianpeng shenxianpeng removed bug Something isn't working documentation Improvements or additions to documentation labels Sep 16, 2026
@2bndy5

2bndy5 commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Does the test PR fail when the actions/checkout ref: ${{ github.event.pull_request.head.sha }} is removed? I'm wondering if that value is different from the default. It is not very cohesive to have users explicitly checkout a PR branch's HEAD.

@2bndy5 2bndy5 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A continuation of my other review comment and my observation in test repo.

Also need to restore the step that does git checkout; actions/checkout default ref value is insufficient for this feature.

Comment thread action.yml Outdated
Comment on lines +562 to +567
# Refresh stat info so untouched files don't show as modified, then stage
# only source files (the configured extensions): anything else an earlier
# step modified stays out of the commit.
^git update-index -q --refresh
let pathspecs = ('${{ inputs.extensions }}' | split row ',' | each { |ext| $"*.($ext | str trim)" })
let changed = (^git ls-files --modified -- ...$pathspecs | lines | where { |line| ($line | str trim | is-not-empty) })

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would seem the AI model does not want to mutate the git staging area. It is going out of its way to avoid git add ...$pathspecs then count the lines in stdout from git status --short.

Suggested change
# Refresh stat info so untouched files don't show as modified, then stage
# only source files (the configured extensions): anything else an earlier
# step modified stays out of the commit.
^git update-index -q --refresh
let pathspecs = ('${{ inputs.extensions }}' | split row ',' | each { |ext| $"*.($ext | str trim)" })
let changed = (^git ls-files --modified -- ...$pathspecs | lines | where { |line| ($line | str trim | is-not-empty) })
# Refresh stat info so untouched files don't show as modified, then stage
# only source files (the configured extensions): anything else an earlier
# step modified stays out of the commit.
^git update-index -q --refresh
let path_specs = ('${{ inputs.extensions }}' | split row ',' | each { |ext| $"*.($ext | str trim)" })
# mutate staging area
^git add ...$path_specs
# count files staged
let changed = (
^git status --short ...$path_specs
| lines
| where { |line| ($line | str trim | is-not-empty) }
| each { |line| ($line | str substring 3..) }
)

If there's no changes to commit, then $changed is empty and the staging area is unaltered. If there are changes then we want to commit them, so the staging area has to be mutated anyway.

Comment thread action.yml Outdated
}
let git_user = $"user.name=($git_user_name)"
let git_email = $"user.email=($git_user_email)"
^git add -- ...$changed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would also be removed since we would do that above (in my other comment/suggestion).

@2bndy5

2bndy5 commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

We might want to also consider an additional input to filter the files in the commit. For projects trying to phase-in clang tooling, this auto-fix-file-filter might be useful. Should probably be a separate issue/PR though.

…er staging

Restore the step that switches from the pull_request merge commit to the
head commit, so workflows can keep the default actions/checkout. It only
acts when HEAD is the merge commit, never forces, and fetches the head
commit only when it is missing: a --depth fetch truncates a full clone.

Stage first and count from `git status --short`, as suggested in review.
With a .gitattributes LF/CRLF rule the old check listed a file that
`git add` normalized away, and `git commit` then failed the step. The
pathspecs are resolved to modified tracked files first because `git add`
aborts on a pathspec that matches nothing, and so that untracked sources
are not committed.
Drop `ref` so the job covers the action's own checkout of the PR head,
and skip the job on fork pull requests, where auto-fix makes no commit.
@github-actions github-actions Bot added bug Something isn't working documentation Improvements or additions to documentation labels Sep 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

support auto-fix option to fix code format automatically in PR?

3 participants